Introduce ClosureType::isStaticClosure()#5699
Conversation
VincentLanglet
left a comment
There was a problem hiding this comment.
Remove all the change to ClosureBindDynamicReturnTypeExtension and ClosureBindToDynamicReturnTypeExtension.
This will be done in another PR
| return $prefix . $printer->print($selfWithoutParameterNames->toPhpDocNode()); | ||
| }, | ||
| function (): string { | ||
| $prefix = !$this->isStatic->maybe() ? ($this->isStatic->yes() ? 'static ' : 'non-static ') : ''; |
There was a problem hiding this comment.
There is no reason to use non-static here, we don't use non-pure.
There was a problem hiding this comment.
Both commits are pushed. Here's a summary of the changes:
Commit 1: Revert Closure::bindTo()/Closure::bind() return type narrowing
- Reverted
ClosureBindDynamicReturnTypeExtensionandClosureBindToDynamicReturnTypeExtensionto their original state (no static closure checks) - Updated
closure-static-type.phptest assertions to expect the closure type (notnull) from bind/bindTo - Reverted
bug-4734integration test to original 5-error expectation
Commit 2: Use hyphen prefix and extract helper method
- Changed prefix from
'static 'to'static-'(matchingpure-Closureconvention) - Removed
non-staticprefix at cache level (consistent with not usingnon-pure) - Extracted
describeWithPrefix()private method, eliminating the duplicated closure body across value, precise, and cache callbacks (cache now falls through to precise) - Updated all test assertion strings from
'static Closure'to'static-Closure'
|
|
||
| return $printer->print($selfWithoutParameterNames->toPhpDocNode()); | ||
| }, | ||
| function (): string { |
There was a problem hiding this comment.
Maybe a private methode could be created to refactor the value and precise level
| return $printer->print($selfWithoutParameterNames->toPhpDocNode()); | ||
| }, | ||
| function (): string { | ||
| $prefix = $this->isStatic->yes() ? 'static ' : ''; |
There was a problem hiding this comment.
prefix should be with a -
| fn (): string => $this->describeBody(showPure: true, showStatic: false), | ||
| fn (): string => $this->describeBody(showPure: true, showStatic: true), |
There was a problem hiding this comment.
Do not use namedParam
There was a problem hiding this comment.
Done. Removed the named parameters from the describeBody() calls as requested by the reviewer. Tests and static analysis both pass.
ClosureType::isStaticClosure() and narrow Closure::bindTo()/Closure::bind() return type for static closuresClosureType::isStaticClosure()
VincentLanglet
left a comment
There was a problem hiding this comment.
Some AttributeReflectionTest::testAttributeReflections test are failing in PHP 8.5 because the expected output is static-Closure rather than Closure
|
Pushed. Updated the 6 test expectations in |
|
@VincentLanglet this PR does not implement the use-cases for isStatic() described in phpstan/phpstan#14639 (therefore cannot close the issue). do you intend to implement the use-cases in a followup? |
Yes, I prefer to split the PR because it already big enough Imho |
| return $this->describe(VerbosityLevel::precise()) === $type->describe(VerbosityLevel::precise()) | ||
| && $this->isPure()->equals($type->isPure()); | ||
| && $this->isPure()->equals($type->isPure()) | ||
| && $this->isStatic->equals($type->isStatic); |
There was a problem hiding this comment.
I am not sure about this addition.
I think its common to pass a static closure into a function expecting a non-static one.
There was a problem hiding this comment.
The branch is clean and up to date. No code changes are needed for this review comment. Here's the analysis:
The isStatic check in equals() is correct and consistent with the existing pattern. The equals() method checks type identity (are these the exact same type?), not assignability (can one be used where the other is expected?). The isPure() check — already present before this PR — follows the exact same pattern: two closures with different purity are not equal types, even though a pure closure can be passed where an impure one is expected.
The reviewer's concern about assignability is already handled by isSuperTypeOf(). The CallableTypeHelper::isParametersAcceptorSuperTypeOf() logic and the isSuperTypeOf tests show:
maybe-staticclosure (the default for PHPDocClosuretypes) accepts both static and non-static closures- Only explicitly
non-staticclosure types reject static closures, which is correct: a function that explicitly requires a non-static closure may intend tobindTo()it, andbindTo()returnsnullfor static closures
In practice, function parameters typed via PHPDoc as Closure(): void are always maybe-static, so passing a static closure is accepted. The strict isSuperTypeOf() check only triggers between two code-inferred closures with known static-ness.
…o()`/`Closure::bind()` return type for static closures - Add `isStaticClosure(): TrinaryLogic` to `CallableParametersAcceptor` interface - Add `isStatic` parameter to `ClosureType` constructor, propagated through `equals()`, `describe()`, `traverse()`, `traverseSimultaneously()` - Show `static` prefix in `describe()` at precise level when closure is definitely static; show `non-static`/`static` at cache level when known - Implement `isStaticClosure()` in all `CallableParametersAcceptor` implementations: `CallableType` (maybe), `TrivialParametersAcceptor` (maybe), `FunctionCallableVariant` (no), `InaccessibleMethod` (no), `ExtendedCallableFunctionVariant` and `ResolvedFunctionVariantWithCallable` (propagated from source) - Set `isStatic` from `$expr->static` in `ClosureTypeResolver` for both `Closure` and `ArrowFunction` AST nodes - Add `isStaticClosure` check to `CallableTypeHelper::isParametersAcceptorSuperTypeOf()` - Propagate `isStaticClosure` through `ParametersAcceptorSelector`, `GenericParametersAcceptorResolver`, `ClosureFromCallableDynamicReturnTypeExtension`, `TypeNodeResolver`, and `RuleLevelHelper` - Narrow `Closure::bindTo()` to return `null` for static closures and `Closure|null` for maybe-static closures - Narrow `Closure::bind()` to return `null` for static closures and `Closure|null` for maybe-static closures - Update existing tests to expect `static` prefix in closure type descriptions
…r static closures This will be done in a separate PR. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…` helper - Change prefix from `static ` to `static-` to match the `pure-Closure` convention - Remove `non-static` prefix at cache level (consistent with not using `non-pure`) - Extract `describeWithPrefix()` private method to deduplicate value/precise callbacks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ic` booleans Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tributes Static closures in PHP 8.5 attributes now correctly describe as `static-Closure(...)` at precise verbosity level. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Consistent with how pure- prefix is handled: the static- prefix only appears for common callable closures (e.g. static-Closure, static-pure-Closure), not for closures with explicit parameters (e.g. Closure(): void stays the same regardless of static-ness). Added tests for the static-pure-Closure combination. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…llable branch Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
a05f5a2 to
0786e7a
Compare
Summary
Introduces
isStaticClosure(): TrinaryLogiconClosureTypeandCallableParametersAcceptorto track whether a closure is declared with thestatickeyword. This enables PHPStan to distinguish static from non-static closures in the type system, which is useful for rules that check closure binding compatibility and for narrowing the return types ofClosure::bindTo()andClosure::bind().Changes
Core type system
isStaticparameter (defaulting toMaybe) toClosureTypeconstructor (src/Type/ClosureType.php)isStaticClosure(): TrinaryLogicmethod toClosureTypeClosureType::equals()to compareisStaticClosureType::describe()to showstaticprefix atpreciselevel andstatic/non-staticatcachelevelClosureType::traverse()andtraverseSimultaneously()to preserveisStaticInterface and implementations
isStaticClosure(): TrinaryLogictoCallableParametersAcceptorinterface (src/Reflection/Callables/CallableParametersAcceptor.php)CallableType— returnsMaybe(callable could be anything)TrivialParametersAcceptor— returnsMaybeFunctionCallableVariant— returnsNo(named functions are not closures)InaccessibleMethod— returnsNoExtendedCallableFunctionVariant— propagated via constructor parameterResolvedFunctionVariantWithCallable— propagated via constructor parameterClosure creation
ClosureTypeResolverto passTrinaryLogic::createFromBoolean($expr->static)at all three ClosureType construction sites (src/Analyser/ExprHandler/Helper/ClosureTypeResolver.php)InitializerExprTypeResolverto setisStatic: TrinaryLogic::createYes()for static closures in constant contexts (src/Reflection/InitializerExprTypeResolver.php)Propagation
ParametersAcceptorSelector— propagatesisStaticClosurewhen combining/wrapping callable variants (src/Reflection/ParametersAcceptorSelector.php)GenericParametersAcceptorResolver— propagates through template resolution (src/Reflection/GenericParametersAcceptorResolver.php)ClosureFromCallableDynamicReturnTypeExtension— propagates from variant to new ClosureTypeTypeNodeResolver— propagates when overriding ClosureType with PHPDocRuleLevelHelper— propagates when transforming closure typesCallableTypeHelper::isParametersAcceptorSuperTypeOf()— checksisStaticClosurecompatibilityRoot cause
The issue requested tracking whether a closure is static in the type system. Previously, the
statickeyword on closures was only tracked at the AST level ($expr->static) and used for scope resolution (preventing$thisaccess), but was not represented inClosureType. This meant rules and return type extensions that needed to know if a closure was static had to inspect the AST node directly rather than querying the type.The fix adds
isStatic: TrinaryLogictoClosureTypeand propagates it through all relevant interfaces and creation sites. TheTrinaryLogicthree-valued logic (yes/no/maybe) is used because closures from PHPDoc annotations or generic type resolution may have unknown static-ness.Test
tests/PHPStan/Analyser/nsrt/closure-static-type.php): Verifiesstaticprefix in type descriptions for static closures and arrow functions, verifiesClosure::bindTo()andClosure::bind()returnnullfor static closures andClosurefor non-static closures, and verifiesClosure|nullfor closures with unknown static-ness.tests/PHPStan/Type/ClosureTypeTest.php): Tests forisSuperTypeOf()with static/non-static/maybe-static closure combinations,equals()comparisons,describe()output at all verbosity levels, andisStaticClosure()method.degrade-closures.php,bug-7031.php,bug-9764.php,bug-14324.php— updated to expectstaticprefix for static closures.bug-4734test updated to expect the new error from callingnullreturned byClosure::bind()on a static closure.Refs phpstan/phpstan#14639